gossip split #6355

endothermicdev · 2023-06-20T20:33:05Z

This PR is step one of removing private gossip from the database.

It removes dependence on private data in the gossip store by logging incoming private gossip to the channels table of the database. The major consumers of this data are invoice and offers, which need to create route hints and blinded routes with this private channel data. The listincoming rpc must therefore be replaced in order to fetch both public and private channel data from the public (topology plugin) and private (lightningd database).

wallet/db.c

gossipd/routing.c

tests/test_gossip.py

plugins/topology.c

vincenzopalazzo · 2023-06-24T11:29:11Z

plugins/topology.c

+	size_t i;
+	const jsmntok_t *t;
+	struct node_id node_id;
+	const struct gossmap_node *node;
+	struct gossmap_chan *chan;
+	int half;
+	struct amount_msat capacity;
+	struct amount_sat chan_capacity_sat;
+	struct amount_msat chan_capacity_msat;
+	u8 *peer_features;
+	const struct gossmap_node *me;
+	u32 my_node_idx;


declare all these fields here looks odds. There is any reason that I am missing here?

vincenzopalazzo · 2023-06-24T11:30:27Z

plugins/topology.c

+			/* If this is a channel with us, get the inbound
+			 * routing parameters. */
+			if(me && json_add_channel_inbound(js, my_node_idx, chan))
+					continue;


the continue has an extra tab?

vincenzopalazzo · 2023-06-24T11:31:03Z

tests/test_plugin.py

+    print(f"ready to test getremoteliquidity({[l3.info['id'], l4.info['id']]})")
+    liquidity_result = l1.rpc.getremoteliquidity([l3.info['id'], l4.info['id']])
+    print(liquidity_result)


same for prints

vincenzopalazzo

I still need to digest this code because the gossipd is always a black box for me, but I had some question and stupid nits

lightningd/gossip_control.c

rustyrussell · 2023-07-11T20:58:35Z

lightningd/gossip_control.c

+	fromwire_gossipd_remote_channel_update(msg, update);
+	channel = any_channel_by_scid(ld, &update->scid, true);
+	if (!channel) {
+		log_unusual(ld->log, "could not find channel for peer's "


log the update source (which peer) and scid?

log_peer_unusual(ld->log, &update->source_node, "Could not find channel %s for peer's channel_update", type_to_string(tmpctx, struct short_channel_id, &update->scid));

rustyrussell · 2023-07-11T20:59:22Z

lightningd/gossip_control.c

+		return;
+	}
+
+	/* FIXME: validate channel belongs to node/peer */


Check channel->peer->id.

lightningd/gossip_control.c

rustyrussell · 2023-07-11T21:03:06Z

lightningd/gossip_control.c

+					   &update->source_node),
+			    type_to_string(tmpctx, struct short_channel_id,
+					   channel->scid));
+		tal_free(channel->private_update);


Don't free real update, do:

if (taken(update)) tal_free(update);

rustyrussell · 2023-07-11T21:05:51Z

gossipd/routing.c

+			/* Allow ld to process a private channel update */
+			struct remote_priv_update remote_update;
+			u8* msg;
+			remote_update.source_node = peer->id;
+			remote_update.scid = short_channel_id;
+			remote_update.fee_base = fee_base_msat;
+			remote_update.fee_ppm = fee_proportional_millionths;
+			remote_update.cltv_delta = expiry;
+			remote_update.htlc_minimum_msat = htlc_minimum;
+			remote_update.htlc_maximum_msat = htlc_maximum;
+			msg = towire_gossipd_remote_channel_update(NULL, &remote_update);
+			daemon_conn_send(peer->daemon->master, take(msg));
+


There are now two of these in gossipd?? Unify into a single ... tell_lightningd_private_cupdate(id, scid, base, ppm, expiry, min, max).?

rustyrussell · 2023-07-11T21:12:47Z

lightningd/gossip_control.c

@@ -554,6 +554,66 @@ static const struct json_command addgossip_command = {
 };
 AUTODATA(json_command, &addgossip_command);

+static struct command_result *json_listprivateinbound(struct command *cmd,


Instead of a new API, perhaps we need to enhance listpeerchannels? (optional) remote_private_update object inside that maybe?

In that case, we need to clean it up once channel is public (probably a good idea anyway!) so gossipd needs to tell is when it (first) makes one of our private channels public (i.e. first sees a valid channel_announcement). Note this can fire more than once (lost gossip store!) so don't freak out if we didn't have private info (could happent if they never sent before announcement anyway!).

rustyrussell · 2023-07-11T21:15:03Z

plugins/topology.c

+		return command_param_failed();
+
+	js = jsonrpc_stream_success(cmd);
+	json_array_start(js, "nodes");


Normally get/listXXX contains XXX object. Shahana has been complaining about inconsistency in our APIs!

rustyrussell · 2023-07-11T21:22:25Z

lightningd/gossip_control.c

+static struct command_result *json_listincoming2(struct command *cmd,
+						 const char *buffer,
+						 const jsmntok_t *obj UNNEEDED,
+						 const jsmntok_t *params)
+{
+	struct peer *peer;


listincoming2 belongs in topology.

It needs to use listpeerchannels to get info about private channels/updates.

Merge that with its topology info (as it does in getremoteliquidity).

Present a unified view of all peers' calculated capacity.

This will be used for generating route hints rather than private gossip

and stash in the database.

lists remote private update inbound routing information for all channels

This allows the removal of private gossip from the gossip store, using the channels db table instead to populate the listincoming private channel entries. Changelog-Changed: Note: Private channels in listincoming deprecated -removal in 24.08.

Changelog-changed: Note: private channels will be deprecated from listchannels in 24.08

rustyrussell

You might have uncovered an unrelated gossip bug!

rustyrussell · 2023-10-16T05:40:14Z

lightningd/gossip_control.c

+	fromwire_gossipd_remote_channel_update(msg, update);
+	channel = any_channel_by_scid(ld, &update->scid, true);
+	if (!channel) {
+		log_unusual(ld->log, "could not find channel for peer's "


log_peer_unusual(ld->log, &update->source_node, "Could not find channel %s for peer's channel_update", type_to_string(tmpctx, struct short_channel_id, &update->scid));

rustyrussell · 2023-10-16T05:40:45Z

lightningd/gossip_control.c

+	case WIRE_GOSSIPD_REMOTE_CHANNEL_UPDATE:
+		/* Please stash in database for us! */
+		handle_private_update_data(gossip->ld, msg);
+		tal_free(msg);


Don't tal_free(msg), it belongs to the caller (and nobody else does!).

rustyrussell · 2023-10-16T05:42:31Z

lightningd/gossip_control.c

+	/* struct node_id *peer_id; */
+	struct peer *peer;
+	/* struct channel *c, **channels; */


Remove comments...

rustyrussell · 2023-10-16T05:42:44Z

lightningd/gossip_control.c

+	response = json_stream_success(cmd);
+	json_array_start(response, "private_channels");
+
+	/* channels = tal_arr(tmpctx, struct channel *, 0); */


rustyrussell · 2023-10-16T05:43:33Z

lightningd/gossip_control.c

+			if (c->state != CHANNELD_NORMAL &&
+			    c->state != CHANNELD_AWAITING_SPLICE)


Prefer new state filters, in this case "if (!channel_can_add_htlc(c->state))".

rustyrussell · 2023-10-16T05:52:35Z

wallet/wallet.c

+		if (local_update->cltv_delta == 0)
+			local_update = tal_free(local_update);


here too...

rustyrussell · 2023-10-16T05:53:01Z

wallet/wallet.c

+		db_bind_null(stmt);
+	}
+	if (chan->local_private_update &&
+	    chan->local_private_update->cltv_delta) {


here too...

rustyrussell · 2023-10-16T05:53:11Z

wallet/wallet.c

@@ -2273,18 +2309,32 @@ void wallet_channel_save(struct wallet *w, struct channel *chan)
 		db_bind_null(stmt);

 	db_bind_int(stmt, chan->ignore_fee_limits);
-	if (chan->private_update) {
+	if (chan->private_update && chan->private_update->cltv_delta) {


rustyrussell · 2023-10-16T05:54:48Z

plugins/topology.c

@@ -22,6 +22,8 @@ static struct gossmap *global_gossmap;
 static struct node_id local_id;
 static struct plugin *plugin;

+extern bool deprecated_apis;


This is already declared in libplugin.h?

rustyrussell · 2023-10-16T05:55:03Z

plugins/topology.c

+	/* FIXME: deprecate private channels in listchannels result in 24.08 */
+	if (deprecated_apis)
+		return;
+


Is this backwards? I think a simple test is in order...

rustyrussell · 2023-10-16T10:31:00Z

OK, I tried eliminating gossipd's private channels, and it's clear that this doesn't go far enough. topology still relies on private channels in gossmap :(

In particular, listprivateinbound is the wrong thing. You need to add the private update information to listpeerchannels, where the channel is not public. Then, instead of using listprivateinbound and doing a lookup in gossmap, use private listpeerchannels. i.e. this code must not use gossmap at all (except, perhaps, to make sure it only shows a private channel if it doesn't appear in gossmap, to avoid races).

rustyrussell · 2023-12-14T23:11:37Z

Parts of this became the gossip rewrite work...